refactor(harness): extract retained workspace scope owners (SAP-3091) - #909
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (31)
💤 Files with no reviewable changes (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change moves workspace scope contracts, path handling, and catalog logic into dedicated modules. Server invalidation and refresh flows use the new workspace helpers. Harness web imports and aliases now reference the new workspace-scope module. ChangesWorkspace scope refactor
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: Merge Risk: ⚪ Minimal · up to This refactor preserves the workspace scope and inventory shapes while moving their ownership to dedicated modules. Path-boundary and catalog coverage accompany the migration, and no concrete merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 27 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit hops where scope keys gleam Comment |
gwitwer
left a comment
There was a problem hiding this comment.
Approving. Compared the moved bodies against the originals: identical logic, identifier renames only. WorkspaceScopeSummary export repointed with an unchanged shape; the @shared alias is updated consistently across tsconfig, vite and vitest; the removed test cases are exactly the ones re-homed into the three new files.
Preserve the reviewed history while updating the stack for review fixes.
Primary change type
Problem and motivation
Workspace identity, allowed-root resolution and per-agent invocation types were still owned by mixed legacy graph modules. Deleting those modules directly would remove contracts used by current Studio discovery and Canvas.
Summary and scope
Extract workspace scope types and local identity helpers into shared/workspace-scope.ts, the catalog into core/workspace-scope-catalog.ts, and containment helpers into core/workspace-path.ts. Move AgentInvocationMode to Canvas ownership. Repoint live imports, browser aliases and the existing public type export; temporary legacy re-exports keep this boundary buildable.
Related work
Related issue or discussion: SAP-3091. Part of the authorized cleanup stack above #892 and #893, targeting
main. The seven layers are intended to merge and ship together. Updated layers require maintainer approval before merge.Validation
Review-fix verification on cumulative source
5bfed981, including currentmain(8679d745). Server and browser typechecks also passed on each of the seven isolated PR heads.The browser override selects the installed Google Chrome, temporary output paths and
pnpm exec vite; it preserves the checked-in tests and assertions. All 16 focused map/style/project-tab cases passed, including light/dark computed styles, project/agent viewport restoration, offscreen recovery, automatic fitting, auth isolation and layout recovery. The viewport and metadata regressions failed before their fixes. The new viewport and project-altitude tests also passed on #907's own intermediate tree (10 cases).An initial three-worker Harness run failed one archive-wiring case; the affected file passed unchanged 8/8, followed by the complete passing two-worker run above. The
mainintegration passed 263 focused cases, and #892's resolved auth mock passed 4/4 on its own tree. No tests or admission guards were weakened.All PR heads include
main's deliberate removal of the duplicate Claude workflow in #886. The reviewed commits remain in history; the stack update uses ordinary fast-forward pushes. Fresh build hashes and the earlier package/smoke evidence are recorded in the retirement record. Signed macOS installer/upgrade validation remains SAP-3086.CI follow-up: the first Node 20 run hit the same archive-wiring failure noted above and cancelled Node 22 through matrix fail-fast. The unchanged rerun passed on both Node 20 and Node 22. The archive test, reader, writer and event store match
mainbyte-for-byte.Tests and documentation
Retained workspace identity, catalog, source containment, public export and per-agent Canvas tests remain at their neutral owners. The moved logic is unchanged.
Compatibility and release impact
Security
AI assistance
Codex implemented the cleanup and test/doc migrations, traced retained callers, and checked the resulting diffs. Automated checks above and independent read-only review provide implementation evidence; maintainer review is still required.
Checklist
CONTRIBUTING.md, and this contribution follows the direct-PR or issue-first policy.